@filiphr I was about to submit such PR, so thanks for creating it.

I think I would prefer to have consistency between mcp-json-jackson2 and mcp-json-jackson3 package naming strategy (version-less or with version), see my related #562 (comment) comment, but that would be a breaking change, so let see with the team what is the preferred solution.

Thanks for your comment @sdeleuze. I've read your comment #562 (comment) and I've added an alternative idea in #562 (comment).

In any case, I stumbled onto this since I wanted to contribute Jackson 3 support for Spring AI, but that depends on this project, so I started here first.

@filiphr Thanks, just be aware I am already working on the Jackson 3 support for Spring AI, and have it pretty advanced locally, so no need to contribute it. That said, I will welcome your review on my upcoming PR.

After the discussion on #562, my proposal is the following:

0.18:

• Introduce mcp-json-jackson3 as implemented in this PR
• In mcp-json-jackson2, deprecate classes for removal in io.modelcontextprotocol.json.jackson/io.modelcontextprotocol.json.schema.jackson and copy them to io.modelcontextprotocol.json.jackson2/io.modelcontextprotocol.json.schema.jackson2 in non-deprecated form.

0.19:

• Remove deprecated classes in io.modelcontextprotocol.json.jackson/io.modelcontextprotocol.json.schema.jackson from mcp-json-jackson2

@filiphr @tzolov @chemicL @graemerocher @sdelamo Are you ok with that? If we are, this PR could be refined with the proposed mcp-json-jackson2 changes for 0.18.

I'm OK with your proposal @sdeleuze. In an hour or two I can adjust this PR and the the changes in relation to mcp-json-jackson2.

@filiphr Thanks, just be aware I am already working on the Jackson 3 support for Spring AI, and have it pretty advanced locally, so no need to contribute it. That said, I will welcome your review on my upcoming PR.

Thanks for letting me know @sdeleuze. I have not spend a lot of time on it anyways, I stumbled fast on the different dependencies. I would be happy to review your PR for Spring AI, feel free to ping me when you have it ready.

After the discussion on #562, my proposal is the following:

0.18:

• Introduce mcp-json-jackson3 as implemented in this PR
• In mcp-json-jackson2, deprecate classes for removal in io.modelcontextprotocol.json.jackson/io.modelcontextprotocol.json.schema.jackson and copy them to io.modelcontextprotocol.json.jackson2/io.modelcontextprotocol.json.schema.jackson2 in non-deprecated form.

0.19:

• Remove deprecated classes in io.modelcontextprotocol.json.jackson/io.modelcontextprotocol.json.schema.jackson from mcp-json-jackson2

@filiphr @tzolov @chemicL @graemerocher @sdelamo Are you ok with that? If we are, this PR could be refined with the proposed mcp-json-jackson2 changes for 0.18.

@sdeleuze @filiphr

This plan makes sense. Please go ahead

Thanks for the review. @sdeleuze @tzolov I have implemented the plan like we discussed.

Thanks everyone for the review. I had to do some additional changes to make sure everything works with Jackson 3.

• Upgrade Spring Framework to 7.x. Otherwise, the webmvc / webflux support does not work with Jackson 3.
• In io.modelcontextprotocol.json.jackson3.JacksonMcpJsonMapper catch the JacksonException and throw IOException. Otherwise, the webflux tests and most likely some more were failing. In Jackson 3 the base JacksonException no longer extends IOException and it's a runtime exception. I do not like this that much, the alternative would be to go through all the places that are calling the methods of McpJsonMapper and adjust them to capture Exception instead of IOException. Another alternative would be to not throw IOException, but rather have a dedicated McpJsonException and each underlying library can throw that if there is some error in the mapping.

As a follow up, we should look into unifying the tests between modules as currently they're a copy.

I agree with you @chemicL it would be good to have some unified tests, maybe tests where all the different json mappers are tested. Btw, I also had some problems with some of the tests in StdioMcpAsyncClientTests and StdioMcpSyncClientTests with errors like:

java.lang.AssertionError: 
Expecting all elements of:
  [TextContent[annotations=null, text=MCP error -32602: Tool sampleLLM not found, meta=null]]
to satisfy given requirements, but these elements did not:

TextContent[annotations=null, text=MCP error -32602: Tool sampleLLM not found, meta=null]
error: java.lang.AssertionError: 
Expecting actual:
  "MCP error -32602: Tool sampleLLM not found"
to end with:
  "Goodbye, world!"

IMO we can't just bump spring-framework from 6.x to 7.x and render the mcp-spring-* modules unusable by anyone with spring-framework 6. I'll gather ideas and come back with feedback.

What I can suggest is the following:

• rollback mcp-spring/* modules to use spring-framework 6.2.x
• make jackson2 a test-only dependency for mcp-spring/* modules (potentially actually removing test-compile-time dependency on it if mocks are used instead of an actual JacksonMcpJsonMapper in the test for the builder that you needed to modify).

@filiphr Would this work to make the tests pass?

@sdeleuze would you be able to verify that with these changes the java-sdk can be used both with Spring Framework 6 in Spring AI 1.0 and Framework 7 in Spring AI 2.0?

As a next step, we can deprecate mcp-spring/* modules in MCP Java SDK and have the implementation for Spring in the Spring portfolio as suggested in #127

@chemicL thanks for pointing it out. The fact that it does not work with Spring 7 is actually a bug in the mcp-spring implementations. It relies on the underlying Jackson implementation and does not use the McpJsonMapper.

I've updated the PR to also fix the places in the mcp-spring implementations that were not using the McpJsonMapper. I actually thing it was a good exercise to run Jackson 3 with Spring 6 (which does not support Jackson 3 natively).

I think the tests should be passing now.

Thank you for taking my feedback into account.

Please do follow the contributing guidelines - force pushing every change makes reviewing the incremental changes difficult.

Can you elaborate on why in the spring modules using the underlying Jackson implementation and not utilizing the McpJsonMapper is a bug? It works with Spring Framework 6 – what changes in Framework 7 that makes it break and forces us to explicitly invoke the mapping from an Object to the JSON String? The content negotiation mechanism in Spring would still perform a check and could do the mapping for us, so perhaps something else is at play?

Please do follow the contributing guidelines - force pushing every change makes reviewing the incremental changes difficult.

Sorry about that. I didn't realize about the force pushing. I won't do that anymore.

Can you elaborate on why in the spring modules using the underlying Jackson implementation and not utilizing the McpJsonMapper is a bug?

It is a bug because it relies on how the WebClient is configured and / or what is available on the classpath. If a user configures their WebClient to use Jackson 2 then not everything might as expected, as one can see with the tests failing when using Spring 6 with Jackson 3.

The 2 changes I did it more or less aligns with what is being done in other places in the mcp-spring modules e.g.

• WebFluxSseClientTransport
• WebFluxSseServerTransportProvider
• WebFluxStatelessServerTransport
• WebFluxStreamableServerTransportProvider
• WebFluxStreamableServerTransportProvider.WebFluxStreamableMcpSessionTransport
• WebFluxSseServerTransportProvider.WebFluxMcpSessionTransport
• WebMvcSseServerTransportProvider
• WebMvcStreamableServerTransportProvider.WebMvcStreamableMcpSessionTransport

The approach fully decouples how the Spring WebClient is configured and whether or not the users have decided to use Jackson 2 or Jackson 3 in their global configuration.

Thanks for explaining, that does make sense to me. In such case, let's bring back jackson2 to the mcp-spring modules and I think we are good to go.

• rather have a dedicated McpJsonException and each underlying library can throw that if there is some error in the mapping.

A dedicated McpJsonException sounds good to me.